Skip to content

docs: describe WFP denial capture - #1420

Open
Richie Gomez (richiemsft) wants to merge 23 commits into
mainfrom
richiemsft/wfp-learning-mode-docs
Open

Richie Gomez (richiemsft) wants to merge 23 commits into
mainfrom
richiemsft/wfp-learning-mode-docs

Conversation

@richiemsft

@richiemsft Richie Gomez (richiemsft) commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

📖 Description

Documents WFP denial capture, runtime compatibility probing, and the caller-facing network denial contract.

  • Distinguishes option-aware native network capture from legacy access-only capture.
  • Documents the exact public source, reason, direction, endpoint, and filterId representation.
  • Clarifies that Tessera is an internal component name and is not exposed in public JSON.
  • Records validated OS build points and guarded fallback behavior.

This is PR 4 of 4 in the WFP Learning Mode stack. Its base is the integration-layer branch.

🔗 References

Related to #1286.

Depends on the preceding integration PR in this stack.

🔍 Validation

  • git diff --check across the complete stack — passed.
  • Reviewed against the final API, decoder, and integration behavior.

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task

🧱 Stack

  1. feat: prefer option-aware Learning Mode trace startup #1417 — API startup and compatibility
  2. feat: decode WFP Learning Mode network events #1418 — WFP event decoding
  3. fix: route captured network decisions through Tessera #1419 — ProcessContainer integration
  4. docs: describe WFP denial capture #1420 — Documentation

Review and merge in this order.

Microsoft Reviewers: Open in CodeFlow

@richiemsft
Richie Gomez (richiemsft) requested a review from a team as a code owner October 6, 2026 20:38
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@MGudgin Gudge (MGudgin) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified review notes

The updated guide correctly distinguishes options-capable native capture from legacy access-only capture and explains why guarded WPR excludes brokered WFP network records. The two inline comments identify a newly incorrect .NET API reference and the undocumented PSEC capture-mode capability exception. This is a comment-only review.

Finding outside the diff

Low (documentation consistency) — Update the README's native/fallback shorthand. README.md:265 — Attribution: newly_exposed_by_change. The README is byte-identical between this PR's base and head, so the earlier wording is not presented as code changed here. This PR's new host-selection guide recognizes compatible legacy-start PSEC hosts as native access-only captures; the README still describes 'complete PSEC/V2' native hosts versus 'older' guarded-WPR tiers, leaving readers of the two linked documents with conflicting classifications. Fix: Align the README's audit-mode summary with the new compatible-export rule and distinguish legacy native capture from guarded WPR.

Verified pre-existing — not attributed to this PR

README.md and docs/process-container/networking.md are both byte-identical from base to head. Their old wording is relevant only insofar as the new guide introduces a conflicting host-selection description or newly documents capture behavior without qualifying the existing capability matrix. No unrelated pre-existing issue is attributed to this documentation PR.

Comment thread docs/logging-access-denied.md Outdated
Comment thread docs/logging-access-denied.md
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-integration branch from 731c328 to cd9e80a Compare October 7, 2026 18:37
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-docs branch from 070db2c to 249d91b Compare October 7, 2026 18:37
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-integration branch 2 times, most recently from cd9e80a to 348dcc1 Compare October 7, 2026 18:45
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-docs branch from 249d91b to 131e258 Compare October 7, 2026 18:45
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-integration branch from 348dcc1 to cdbd9d6 Compare October 7, 2026 20:27
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-docs branch from 131e258 to 6c29917 Compare October 7, 2026 20:27
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-integration branch from cdbd9d6 to 82dce5d Compare October 7, 2026 20:37
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-docs branch from 6c29917 to fb62dab Compare October 7, 2026 20:37
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-integration branch from 82dce5d to 72c85b5 Compare October 7, 2026 22:36
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-docs branch from fb62dab to 6707972 Compare October 7, 2026 22:36
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-integration branch from 72c85b5 to 67e25e8 Compare October 8, 2026 17:29
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-docs branch 2 times, most recently from f0432aa to 92693ee Compare October 8, 2026 17:36
@richiemsft

Copy link
Copy Markdown
Contributor Author

Gudge (@MGudgin) The documentation findings are fixed in 92693ee: the V1 .NET type names are restored, and both the denial-capture guide and ProcessContainer networking matrix explain the narrowly scoped native PSEC capture capability exception. The docs are restacked on the implementation fix. Please re-review the updated tip.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Several documented compatibility, artifact-version, and incomplete-endpoint behaviors conflict with the implementation.

3 open findings
What changed in this PR

Documents WFP network-denial capture and its compatibility, output, and telemetry contracts.

Changes:

  • Describes option-aware versus legacy capture.
  • Documents network-denial records and verbose v5 diagnostics.
  • Records ProcessContainer networking and OS compatibility behavior.
File Description
docs/​logging-access-denied.md Expands denial-capture and artifact contracts.
docs/​development/​architecture/​telemetry.md Documents verbose telemetry projection.
docs/​backends/​process-container/​os-version-support.md Adds runtime compatibility guidance.
docs/​backends/​process-container/​networking.md Explains capture-only internetClient.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/logging-access-denied.md
Comment thread docs/logging-access-denied.md Outdated
Comment thread docs/logging-access-denied.md Outdated
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-docs branch from 0ef5d02 to 0613f58 Compare October 8, 2026 19:26
@richiemsft
Richie Gomez (richiemsft) force-pushed the richiemsft/wfp-learning-mode-integration branch from 7f147b9 to 6150aa6 Compare October 8, 2026 19:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Two statements misdocument Tessera attribution and the verbose schema version emitted by legacy native capture.

2 open findings
3 resolved since last review

🧠 Review effort: Balanced

Comment thread docs/logging-access-denied.md Outdated
Comment thread docs/logging-access-denied.md Outdated
Document the source-domain and WFP provider/sublayer identity required before interpreting Tessera reasons. Clarify that both option-aware and legacy access-only native capture emit verbose version 5, while guarded WPR emits version 4.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Copilot AI balanced review requested due to automatic review settings October 14, 2026 17:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The documentation omits several serialized reason values and contradicts itself about actionable network denials.

0 open findings

2 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Low severity Clarify schema-failure behavior versus actionable network denials

docs/​logging-access-denied.md:269

This newly documented actionable network-denial behavior contradicts the existing schema-failure paragraph at lines 517–520, which still says “network decisions are not denials.” The implementation records network-provider schema failures diagnostically without setting deniedResourcesTruncated; update that paragraph to describe this behavior without denying the actionable contract introduced here.

Low severity Document all networkDecisionReason field values

docs/​logging-access-denied.md:286

The table does not document the serialized networkDecisionReason contract promised by the PR: only directDefaultDeny appears later in an example, leaving consumers unable to discover appIsolationMissingCapability, authoredExplicitDeny, allowExclusion, and proxyContainment. Add the exact field values for every row.

🧠 Review effort: Balanced

List every serialized networkDecisionReason value. Clarify network-provider schema failures do not set deniedResourcesTruncated, without contradicting actionable decoded network denials or the guarded-WPR exclusion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Copilot AI balanced review requested due to automatic review settings October 14, 2026 18:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The documentation does not explicitly state the promised internal-only status of the Tessera name.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Low severity Clarify Tessera is internal-only and absent from public JSON

docs/​logging-access-denied.md:269

The PR description promises to clarify that Tessera is only an internal component name and is not exposed in public JSON, but this section introduces Tessera as a source domain without stating that boundary. This can make readers infer a caller-facing source: "Tessera" field even though the public record below has no such field. Please make the internal-only status explicit here.

🧠 Review effort: Balanced

@richiemsft

Copy link
Copy Markdown
Contributor Author

Addressed both previously missed findings from the latest Copilot review in f04896c. The source/reason table now lists all five serialized networkDecisionReason values. The schema-failure paragraph now states that network-provider schema failures are diagnostic-only and do not set deniedResourcesTruncated, while successfully decoded network events may produce actionable denials. The adjacent stale unsupportedEventSchema wording was also corrected to match native decoding and guarded-WPR scoping.

Explicitly distinguish the internal component name from the public denial record and verbose JSON provider/reason vocabulary.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Copilot AI balanced review requested due to automatic review settings October 14, 2026 18:31
@richiemsft

Copy link
Copy Markdown
Contributor Author

Addressed the latest summary-only Copilot finding in 0ed5164. The network-denial section now explicitly identifies Tessera as an internal Windows component name, states that it is not exposed in public JSON, and distinguishes the source-free actionable record from the learningModeNetworkDecision provider and typed networkDecisionReason vocabulary in verbose logging.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The updated behavior conflicts with existing proxy and ETL-retention guidance in docs/schema.md.

0 open findings

Previously missed (2)

In code that hasn't changed since last review

Low severity Correct schema guidance for native capture with network proxy

docs/​logging-access-denied.md:258

This proxy-mode behavior conflicts with docs/schema.md:259-261, which still says native capture cannot be combined with runtimeConfig.networkProxy. The implementation explicitly accepts capture with a runtime proxy (base_container_runner.rs:3836-3845) while preserving the proxy capability posture (base_container_runner.rs:3128-3153). Please update that schema guidance so callers are not told this supported combination will fall back or fail.

Low severity Update schema guidance for guarded-WPR retention fallback

docs/​logging-access-denied.md:582

The canonical configuration guide still states that the guarded-WPR fallback rejects retention (docs/schema.md:255-256), contradicting this new guarantee. The product PlmGuardedCaptureFactory enables trace transfer (src/mxc-sdk/src/core/mxc_engine/guarded_capture.rs:203-210), so please update the schema guidance to document the supported fallback behavior consistently.

🧠 Review effort: Balanced

Document supported native runtime proxy capture and workload-filtered guarded-WPR retention. Preserve least-privilege and other proxy fallback guidance and replace stale PSEC/V2-only wording.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Copilot AI balanced review requested due to automatic review settings October 14, 2026 20:18
@richiemsft

Copy link
Copy Markdown
Contributor Author

Addressed both previously missed schema-guide findings in 7df6c67. docs/schema.md now documents native PSEC capture with runtimeConfig.networkProxy while preserving the proxy capability posture and leastPrivilege/other-proxy fallback. It also documents guarded-WPR retention of a workload-process-filtered ETL, never the host-wide source, and replaces stale PSEC/V2-only guidance.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The version-5 artifact example omits required retained properties without identifying itself as abridged.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Low severity Artifact omits required NetworkDecisionV1 properties

docs/​logging-access-denied.md:429

This complete-looking artifact cannot be emitted as shown. An accepted NetworkDecisionV1 must contain all 24 schema properties, and the producer sanitizes the full property list; only timestamp-like entries such as OriginalTimestamp are dropped, so the resulting properties array contains many more entries than ApplicationId and FilterId. Please either show the complete retained array or explicitly mark this as an abridged, non-literal excerpt so consumers do not use it as the expected producer output.

🧠 Review effort: Balanced

Clarify that the shortened properties array is illustrative rather than literal producer output or a fixture, and that accepted events retain sanitized schema properties subject to documented omissions and bounds.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Copilot AI balanced review requested due to automatic review settings October 14, 2026 20:29
@richiemsft

Copy link
Copy Markdown
Contributor Author

Addressed the latest summary-only Copilot finding in cb48248. The version-5 JSON example is now explicitly labeled an abridged, non-literal illustration, not complete producer output or an expected-output fixture. The note explains that only two properties are shown and that accepted NetworkDecisionV1 events contain all 24 schema properties, retained after the documented sanitization, omissions, and bounds.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The public IPv4 and IPv6 ICMP resource representations remain ambiguous.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Low severity Document distinct ICMP and ICMPv6 endpoint representations

docs/​logging-access-denied.md:367

The ICMP endpoint contract is still ambiguous here because the two public representations intentionally differ. An actionable IPv4 denial uses resource: "icmp://…", an IPv6 denial uses resource: "icmpv6://[…]", but the version-5 networkEndpoint.protocol and the authored policy both use "icmp" for either family (network_extractors.rs:400-433, 751-775). Document both resource schemes so consumers do not copy icmpv6 into the policy protocol or parse IPv6 records as icmp://.

🧠 Review effort: Balanced

Document portless IPv4 icmp and IPv6 icmpv6 resource representations while preserving the shared icmp protocol in structured endpoints and authored policy.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Copilot AI balanced review requested due to automatic review settings October 14, 2026 21:09
@richiemsft

Copy link
Copy Markdown
Contributor Author

Addressed the latest Copilot summary finding in 46273a1. The guide now explicitly distinguishes portless IPv4 icmp://203.0.113.10 and IPv6 icmpv6://[2001:db8::1] resources. Both structured networkEndpoint.protocol and the authored policy use icmp, with no port field; the text warns not to copy icmpv6 into policy protocol.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The documentation matches the implementation, and no unresolved issues remain.

0 open findings

🧠 Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants